Skip to content

fix(binding): stop memoizing a failed ICreatesObservableForProperty lookup - #58

Merged
glennawatson merged 1 commit into
mainfrom
fix/issue-31-observable-for-property-lookup
Aug 2, 2026
Merged

fix(binding): stop memoizing a failed ICreatesObservableForProperty lookup#58
glennawatson merged 1 commit into
mainfrom
fix/issue-31-observable-for-property-lookup

Conversation

@glennawatson

Copy link
Copy Markdown
Contributor

What kind of change does this PR introduce?

Bug fix (runtime property observation lookup).

What is the new behavior?

  • A failed ICreatesObservableForProperty lookup is no longer remembered. If nothing is registered when a property is first observed, a later lookup for the same property re-scans the locator and succeeds once registration has happened.
  • Only successful resolutions are written to the cache, so the bounded MRU still does its job on the hit path.
  • The hot path got faster as a side effect: reading through TryGet instead of Get measures 18.94 ns against 45.02 ns, a ratio of 0.42, because supplying the resolved value through the cache context removes a second locator scan on the write path.

What is the current behavior?

Closes #31

The resolution cache is keyed by sender type, property name and before-change flag, and its factory returns null when nothing in the locator bids a positive affinity. That null is memoized permanently and nothing invalidates it. A single lookup that happens before registrations land therefore disables that key for the lifetime of the process, and every later call throws Could not find a ICreatesObservableForProperty even though registration has since completed correctly. A different property on the same type continues to work, which is why the failure looks intermittent and is most visible inside a test run where initialization order varies.

What might this PR break?

  • None expected. A lookup that previously threw may now succeed, which is the point. Successful resolutions are cached exactly as before, and the MRU bound is unchanged.
  • The cache write no longer happens under a single combined operation. TryGet and Get are each internally gated, and the only race left is a redundant re-resolve of the same key, never an incorrect result.

Checklist

  • I have read the Contribute guide
  • Tests have been added or updated (for bug fixes / features)
  • Docs have been added or updated (for bug fixes / features)
  • Changes target the main branch
  • PR title follows Conventional Commits

Additional information

Evidence for the diagnosis. A throwaway app against the runtime project, doing lookup then registration then lookup, produced:

before registration:                       THREW -> Could not find a ICreatesObservableForProperty
registered ICreatesObservableForProperty count: 2
after registration (poisoned key 'Name'):  THREW -> Could not find a ICreatesObservableForProperty
after registration (fresh key 'Count'):    OK

Two regression tests cover the by-name and expression paths. Both were confirmed failing against the unmodified runtime on net8, net9, net10 and net11 using the committed test bodies, and passing with the change.

Ruled out while investigating: AppLocator.Current and Locator.Current are the same instance, so there is no split between the store this library reads and the one others register into; and the generator does not intercept ReactiveUI's own WhenAnyValue, whose containing type is not one of the recognised extension classes.

Two related problems were found and are deliberately not addressed here, since they are separate from the caching defect:

  1. A published ReactiveUI.Binding can force a Splat assembly-version downgrade that stops ReactiveUI binding to Splat at all, which produces a different failure with the same "add these two packages, remove them again" shape.
  2. Nothing on the runtime fallback path calls the builder's initialization guard, so a consumer who never initializes gets the "your service locator is probably broken" message rather than the actionable guidance the builder already has.

Build is clean with 0 warnings and 0 errors. The full suite passes 10764/10764, run twice, with the runtime test project run three further times on its own to check for order dependence.

…ookup

- Resolve the factory through a helper that reads the MRU cache with TryGet and
  only writes back a resolution that found an implementation.
- A lookup made while the locator is still empty previously stuck for the life of
  the process, so that sender and property kept throwing "Could not find a
  ICreatesObservableForProperty" long after registration had happened.
- Supply the entry through the cache's context argument so the write costs no
  second locator scan; the read path drops from 45.0ns to 18.9ns.
- Cover both the by-name and the expression-chain path with a lookup that runs
  before registration and succeeds after it.
@codecov

codecov Bot commented Aug 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.61%. Comparing base (8565c11) to head (a1c4e14).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #58   +/-   ##
=======================================
  Coverage   97.61%   97.61%           
=======================================
  Files         227      227           
  Lines        7744     7744           
  Branches     1062     1062           
=======================================
  Hits         7559     7559           
  Misses        143      143           
  Partials       42       42           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@sonarqubecloud

sonarqubecloud Bot commented Aug 2, 2026

Copy link
Copy Markdown

@glennawatson
glennawatson merged commit c37f8ca into main Aug 2, 2026
12 checks passed
@glennawatson
glennawatson deleted the fix/issue-31-observable-for-property-lookup branch August 2, 2026 03:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: "Could not find a ICreatesObservableForProperty" is thrown under specific circumstances

1 participant